Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for JUnit 6 (Jupiter API 6.x) test framework alongside the existing JUnit 5 support. JUnit 6 requires Java 17+ and uses version 6.x for both junit-jupiter-* and junit-platform-* artifacts, whereas JUnit 5 uses 5.x for Jupiter and 1.x for Platform components.
Key changes include:
- Added
JUnit6enum value toTestKindacross TypeScript and Java codebases - Implemented JUnit 6 detection based on Java version (17+) and junit-jupiter-api version (6.x+)
- Enhanced classpath filtering logic to select appropriate JUnit versions and inject missing bundles
- Updated build scripts to handle both JUnit 5 and 6 versions simultaneously
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/java-test-runner.api.ts | Added JUnit6 enum value to TestKind |
| src/utils/launchUtils.ts | Extended tag parsing logic to include JUnit6 |
| src/runners/junitRunner/JUnitRunnerResultAnalyzer.ts | Updated result analysis to handle JUnit6 uniqueId and displayName |
| src/controller/testController.ts | Added JUnit6 routing to JUnitRunner and version tracking |
| scripts/buildJdtlsExt.js | Implemented multi-version JAR selection to support both JUnit 5 and 6 bundles |
| package.json | Updated to include both JUnit 5 (5.x/1.x) and JUnit 6 (6.x) bundle versions |
| package-lock.json | Version bump to 0.43.2 |
| java-extension/com.microsoft.java.test.target/com.microsoft.java.test.tp.target | Updated Eclipse repository URLs to 4.38 and added junit6 runtime dependency |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/model/TestKind.java | Added JUnit6 enum value with serialization support |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/provider/TestKindProvider.java | Implemented JUnit 6 detection logic based on Java 17+ and junit-jupiter-api 6.x |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/searcher/JUnit6TestSearcher.java | New searcher implementation for JUnit 6 test discovery |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/util/TestFrameworkUtils.java | Added JUnit6TestSearcher registration |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/util/TestGenerationUtils.java | Extended lifecycle annotation handling for JUnit6 |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/util/JUnitPlugin.java | Added OSGi bundle status logging for JUnit debugging |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchUtils.java | Added JUnit6 loader constant |
| java-extension/com.microsoft.java.test.plugin/src/main/java/com/microsoft/java/test/plugin/launchers/JUnitLaunchConfigurationDelegate.java | Implemented classpath filtering and JUnit bundle injection logic for version compatibility |
| java-extension/com.microsoft.java.test.plugin/META-INF/MANIFEST.MF | Removed explicit JUnit bundle dependencies from Require-Bundle (now injected dynamically) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| const junit5Pattern = new RegExp(`^${baseName}_(5\\.\\d+\\.\\d+)\\.jar$`); | ||
| const junit6Pattern = new RegExp(`^${baseName}_(6\\.\\d+\\.\\d+)\\.jar$`); | ||
| const platform1Pattern = new RegExp(`^${baseName}_(1\\.\\d+\\.\\d+)\\.jar$`); |
There was a problem hiding this comment.
The regex patterns only match versions with exactly 3 digits (e.g., 5.11.0 or 6.0.1). This will fail to match versions with 4 segments like 5.11.0.RELEASE or snapshot versions like 6.0.0-SNAPSHOT. Consider using patterns like (5\\.\\d+\\.\\d+[^_]*) or (6\\.\\d+\\.\\d+[^_]*) to handle version qualifiers.
| const junit5Pattern = new RegExp(`^${baseName}_(5\\.\\d+\\.\\d+)\\.jar$`); | |
| const junit6Pattern = new RegExp(`^${baseName}_(6\\.\\d+\\.\\d+)\\.jar$`); | |
| const platform1Pattern = new RegExp(`^${baseName}_(1\\.\\d+\\.\\d+)\\.jar$`); | |
| const junit5Pattern = new RegExp(`^${baseName}_(5\\.\\d+\\.\\d+[^_]*)\\.jar$`); | |
| const junit6Pattern = new RegExp(`^${baseName}_(6\\.\\d+\\.\\d+[^_]*)\\.jar$`); | |
| const platform1Pattern = new RegExp(`^${baseName}_(1\\.\\d+\\.\\d+[^_]*)\\.jar$`); |
| const parentIndex: string = result[5]; | ||
| const displayName: string = result[6].replace(/\\,/g, ','); | ||
| const uniqueId: string | undefined = this.testContext.kind === TestKind.JUnit5 ? | ||
| const uniqueId: string | undefined = (this.testContext.kind === TestKind.JUnit5 || this.testContext.kind === TestKind.JUnit6) ? |
There was a problem hiding this comment.
The condition (this.testContext.kind === TestKind.JUnit5 || this.testContext.kind === TestKind.JUnit6) is repeated multiple times in this file (lines 352 and 401). Consider extracting this into a helper method or property like isJupiterBasedTest() to improve maintainability and reduce code duplication.
| args: [ | ||
| ...launchArguments.programArguments, | ||
| ...(testContext.kind === TestKind.JUnit5 ? parseTags(config) : []) | ||
| ...(testContext.kind === TestKind.JUnit5 || testContext.kind === TestKind.JUnit6 ? parseTags(config) : []) |
There was a problem hiding this comment.
The condition (testContext.kind === TestKind.JUnit5 || testContext.kind === TestKind.JUnit6) appears multiple times across the codebase. Consider creating a helper function like isJupiterTest(kind: TestKind) or adding a method to the TestKind enum to check if a test kind is Jupiter-based (JUnit 5 or 6), improving code maintainability.
| "./server/org.eclipse.jdt.junit4.runtime_1.4.0.v20251113-1434.jar", | ||
| "./server/org.eclipse.jdt.junit5.runtime_1.2.0.v20251113-1434.jar", | ||
| "./server/org.eclipse.jdt.junit6.runtime_1.0.0.v20251112-1701.jar", |
There was a problem hiding this comment.
[nitpick] The ordering of javaExtensions has changed - the Eclipse runtime bundles (org.eclipse.jdt.junit*.runtime) now appear after JUnit artifacts instead of before them. While this may not affect functionality, maintaining a consistent grouping (e.g., Eclipse bundles together, then JUnit artifacts) would improve maintainability. Consider grouping by: 1) JUnit artifacts, 2) Common dependencies (opentest4j, apiguardian), 3) Eclipse bundles, 4) Other bundles.
|
|
||
| private static int getMajorVersion(String version) { | ||
| if (version.startsWith("1.")) { | ||
| return Integer.parseInt(version.substring(2, 3)); |
There was a problem hiding this comment.
The version parsing logic for Java 1.x format (e.g., "1.8") is incorrect. It uses substring(2, 3) which only extracts a single digit, failing for version "1.10" or higher. It should use parseInt(version.substring(2)) to parse the entire minor version after "1.".
| return Integer.parseInt(version.substring(2, 3)); | |
| return Integer.parseInt(version.substring(2)); |
| } | ||
|
|
||
| final String artifactName = matcher.group(1); | ||
| final int majorVersion = Integer.parseInt(matcher.group(2)); |
There was a problem hiding this comment.
Potential uncaught 'java.lang.NumberFormatException'.
| final int majorVersion = Integer.parseInt(matcher.group(2)); | |
| final String versionGroup = matcher.group(2); | |
| int majorVersion; | |
| try { | |
| majorVersion = Integer.parseInt(versionGroup); | |
| } catch (NumberFormatException e) { | |
| JUnitPlugin.logInfo("[Classpath Filter] " + testKind + " - Skipping: " + fileName + " (invalid version: " + versionGroup + ")"); | |
| excludedCount++; | |
| continue; | |
| } |
| return Integer.parseInt(version.substring(2, 3)); | ||
| } | ||
| final int dot = version.indexOf('.'); | ||
| if (dot != -1) { | ||
| return Integer.parseInt(version.substring(0, dot)); |
There was a problem hiding this comment.
Potential uncaught 'java.lang.NumberFormatException'.
| return Integer.parseInt(version.substring(2, 3)); | |
| } | |
| final int dot = version.indexOf('.'); | |
| if (dot != -1) { | |
| return Integer.parseInt(version.substring(0, dot)); | |
| try { | |
| return Integer.parseInt(version.substring(2, 3)); | |
| } catch (NumberFormatException | IndexOutOfBoundsException e) { | |
| return 0; | |
| } | |
| } | |
| final int dot = version.indexOf('.'); | |
| if (dot != -1) { | |
| try { | |
| return Integer.parseInt(version.substring(0, dot)); | |
| } catch (NumberFormatException e) { | |
| return 0; | |
| } |
| } | ||
| final int dot = version.indexOf('.'); | ||
| if (dot != -1) { | ||
| return Integer.parseInt(version.substring(0, dot)); |
There was a problem hiding this comment.
Potential uncaught 'java.lang.NumberFormatException'.
| return Integer.parseInt(version.substring(0, dot)); | |
| try { | |
| return Integer.parseInt(version.substring(0, dot)); | |
| } catch (NumberFormatException e) { | |
| return 0; | |
| } |
| final int major = Integer.parseInt(m.group(1)); | ||
| if (major >= 6) { | ||
| return true; |
There was a problem hiding this comment.
Potential uncaught 'java.lang.NumberFormatException'.
| final int major = Integer.parseInt(m.group(1)); | |
| if (major >= 6) { | |
| return true; | |
| try { | |
| final int major = Integer.parseInt(m.group(1)); | |
| if (major >= 6) { | |
| return true; | |
| } | |
| } catch (NumberFormatException e) { | |
| // skip this entry if version is not a valid integer |
| function selectJarVersion(files, baseName) { | ||
| const versions = selectJarVersions(files, baseName); | ||
| return versions.length > 0 ? versions[0] : null; | ||
| } |
There was a problem hiding this comment.
Unused function selectJarVersion.
| function selectJarVersion(files, baseName) { | |
| const versions = selectJarVersions(files, baseName); | |
| return versions.length > 0 ? versions[0] : null; | |
| } |
Bundles required for JUnit 6
{"junit-jupiter-api", "[6.0.0,7.0.0)"} {"junit-jupiter-engine", "[6.0.0,7.0.0)"} {"junit-jupiter-params", "[6.0.0,7.0.0)"} {"junit-platform-commons", "[6.0.0,7.0.0)"} {"junit-platform-engine", "[6.0.0,7.0.0)"} {"junit-platform-launcher", "[6.0.0,7.0.0)"} {"junit-platform-suite-api", "[6.0.0,7.0.0)"} {"junit-platform-suite-engine", "[6.0.0,7.0.0)"} // common {"org.opentest4j", "[1.0.0,3.0.0)"} {"org.apiguardian.api", "[1.0.0,2.0.0)"}